New - Allow user-provided sampler and add out of the box CEL sampler support#473
Open
cleverchuk wants to merge 1 commit into
Open
New - Allow user-provided sampler and add out of the box CEL sampler support#473cleverchuk wants to merge 1 commit into
cleverchuk wants to merge 1 commit into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the SolarWinds OpenTelemetry distro so users can configure a non-default sampler (including the OpenTelemetry CEL sampler) without the agent disabling itself, and reduces build/startup friction in Lambda and Shadow JAR packaging.
Changes:
- Stop disabling the agent when the configured sampler is not
SolarwindsSampler; only inject the default sampler when none is configured. - Add
opentelemetry-cel-samplerdependency to support CEL sampler configuration out-of-the-box. - Remove the Lambda JDK version gating and suppress ShadowJar duplicate LICENSE/NOTICE warnings.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
libs/shared/src/main/java/com/solarwinds/opentelemetry/extensions/config/provider/SharedConfigCustomizerProvider.java |
Only adds the default sampler when no sampler is configured. |
libs/shared/src/test/java/com/solarwinds/opentelemetry/extensions/config/provider/SharedConfigCustomizerProviderTest.java |
Adds coverage ensuring user-provided sampler config isn’t overridden. |
libs/shared/build.gradle.kts |
Adds CEL sampler dependency (with exclusions). |
dependencyManagement/build.gradle.kts |
Adds managed dependency entry for CEL sampler. |
custom/src/main/java/com/solarwinds/opentelemetry/extensions/config/provider/AutoConfigurationCustomizerProviderImpl.java |
Removes the “ratchet off” setAgentEnabled API. |
custom/src/main/java/com/solarwinds/opentelemetry/extensions/SolarwindsAgentListener.java |
Removes sampler-type enforcement and always runs startup tasks when enabled. |
custom/src/test/java/com/solarwinds/opentelemetry/extensions/provider/AutoConfigurationCustomizerProviderImplTest.java |
Updates tests for removal of setAgentEnabled. |
custom/src/test/java/com/solarwinds/opentelemetry/extensions/SolarwindsAgentListenerTest.java |
Updates tests to validate shutdown behavior based on isAgentEnabled(). |
libs/lambda/src/main/java/com/solarwinds/opentelemetry/extensions/DefaultAutoConfigurationCustomizerProvider.java |
Removes Lambda JDK version check; defaults agent enabled unless config load fails. |
libs/lambda/src/main/java/com/solarwinds/opentelemetry/extensions/LambdaAgentListener.java |
Removes sampler-type enforcement in Lambda listener. |
libs/lambda/src/test/java/com/solarwinds/opentelemetry/extensions/LambdaAgentListenerTest.java |
Updates tests for new enable/disable behavior. |
agent/build.gradle.kts |
Excludes duplicate LICENSE/NOTICE files during Shadow relocation step. |
agent-lambda/build.gradle.kts |
Same Shadow duplicate LICENSE/NOTICE exclusion for Lambda agent build. |
testing/agent-for-testing/build.gradle.kts |
Same Shadow duplicate LICENSE/NOTICE exclusion for test agent build. |
…/o extension and removed JDK version check in lambda.
cleverchuk
force-pushed
the
cc/NH-132097
branch
from
e8d85fd to
f07cf9a
Compare
|
This PR has been automatically marked as stale because it has not had any activity in the last 7 days. It will be closed in 7 days if no further activity occurs. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Allow users to provide their own sampler (e.g., CEL sampler) instead of forcing the SolarwindsSampler. Remove the JDK version check in the Lambda extension. Fix duplicate META-INF license file warnings in shadow JARs.
Details
User-provided sampler support
Previously, the
AgentListenerimplementations (SolarwindsAgentListener,LambdaAgentListener) checked at startup whether the configured sampler was an instance ofSolarwindsSampler. If not, they disabled the agent entirely and shut down the SDK. This prevented users from using alternative samplers (like the OpenTelemetry CEL sampler) while still benefiting from the rest of the SolarWinds extensions (exporters, propagators, span processors).This change removes the
isUsingSolarwindsSamplerruntime check and thesetAgentEnabledmethod that allowed it to ratchet the agent off. The agent now starts its background tasks (settings reader, HTTP settings delegate, shutdown hooks) wheneverisAgentEnabled()is true, regardless of which sampler is in use.On the declarative configuration side,
SharedConfigCustomizerProvider.addSampler()now checks whether a sampler is already configured before injecting the defaultSolarwindsSampler. If the user has declared a sampler in their config file, it is preserved.The
opentelemetry-cel-samplercontrib library is added as a dependency so users can configure CEL-based sampling rules directly in their OpenTelemetry config file without writing a custom extension.JDK version check removal in Lambda
The Lambda configuration provider previously gated initialization on
JavaRuntimeVersionChecker.isJdkVersionSupported(). This check is no longer needed because AWS Lambda runtimes guarantee a supported JDK version. Removing it simplifies the startup path and eliminates a dependency on the version checker in the Lambda module.META-INF duplicate file fix
Shadow/fat JAR tasks in
agent,agent-lambda, andtesting/agent-for-testingnow useDuplicatesStrategy.EXCLUDEforMETA-INF/LICENSE*andMETA-INF/NOTICE*files to suppress build warnings from multiple dependencies bundling the same license files.Test services data